Fix deadlock crash when updating UserDefaults #16256
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
adborbas
approved these changes
Oct 20, 2025
| // Use dedicated serial queue for UserDefaults operations to: | ||
| // 1. Prevent race conditions where concurrent writes overwrite each other | ||
| // 2. Avoid deadlock by not using the main queue that KVO observers may need | ||
| userDefaultsQueue.async { [weak self] in |
There was a problem hiding this comment.
Small note, as we are using async here notificationCenter might fire before the value is actually set.
There was a problem hiding this comment.
Good point! I updated this in fc11138 and removed the waiting in tests.
| // Use dedicated serial queue for UserDefaults operations to: | ||
| // 1. Prevent race conditions where concurrent writes overwrite each other | ||
| // 2. Avoid deadlock by not using the main queue that KVO observers may need | ||
| userDefaultsQueue.async { [weak self] in |
There was a problem hiding this comment.
Same concerc as above, clearUnsupportedFlag might return befire the value is set in UserDefault
There was a problem hiding this comment.
Also done in fc11138.
…ecessary wait in tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes WOOMOB-1524
Description
There is a crash report about a deadlock caused as part of the error handling when forcing direct requests for sites.
This deadlock is caused by updating
UserDefaultsinside the same queue we use for flagging site with barrier block. To be more specific,UserDefaultsuses Key-Value Observing (KVO), which delivers notifications synchronously. When we write toUserDefaultsinside a barrier block, the KVO observer fires before the barrier completes, and if that observer tries to access the same queue → deadlock.The solution is to fire updates to UserDefaults to a separate serial queue to avoid blocking the concurrent queue we use for error handling.
Testing steps
It's not straightforward to reproduce the deadlock. Simply smoke testing the flow should be sufficient.
Use the test plan in pe5sF9-4Am-p2 - run TC4. Confirm that the app doesn't crash.
Testing information
Screenshots
N/A
RELEASE-NOTES.txtif necessary.